Skip to content

fix(cache): degrade safely when optional cache initialization fails#1890

Merged
openai0229 merged 7 commits into
OtterMind:mainfrom
HandSonic:fix/1724-cache-manage-throwable
Jul 24, 2026
Merged

fix(cache): degrade safely when optional cache initialization fails#1890
openai0229 merged 7 commits into
OtterMind:mainfrom
HandSonic:fix/1724-cache-manage-throwable

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 21, 2026 •
edited by openai0229
Loading

Copy link
Copy Markdown
Contributor

Related issue

Closes #1979

Summary

Makes optional disk-cache initialization a narrow, testable degradation boundary.

  • initialization failures caused by Exception or LinkageError produce an explicit unavailable state
  • fatal non-linkage errors such as OutOfMemoryError, StackOverflowError, and ThreadDeath remain visible to the runtime
  • reads fall back to the existing source functions while caching is unavailable
  • fuzzyDelete and close are safe no-ops without a cache manager

The public static CacheManage API remains unchanged. Cache path, capacity, expiry, and JSON serialization are also unchanged.

Affected surfaces

  • Frontend / Web
  • Backend / API / Storage
  • Database plugin / Driver
  • JCEF / Desktop packaging
  • CI / Build / Release
  • Documentation only

Verification

  • Commands and results:
    • mvn -pl chat2db-community-domain/chat2db-community-domain-core -am -Dtest=CacheManageTest -Dsurefire.failIfNoSpecifiedTests=false -Dmaven.test.skip=false -DskipTests=false test - passed; 4 tests, 0 failures/errors/skips
    • git diff --check origin/main...HEAD - passed
  • Manual verification: N/A; failure modes are injected at the cache-manager construction boundary.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: Public static methods and on-disk cache format remain unchanged.
  • Database or driver compatibility: N/A.
  • Network, privacy, or security: N/A; local optional metadata cache only.
  • Community / Local / Pro boundary: Community domain cache implementation only.
  • Backward compatibility: Existing cache hits, refreshes, fallback functions, invalidation, and serialization are preserved.

Reviewer map

  • Start here: CacheManage.CacheStore.initialize, then CacheManageTest.
  • Failure condition: Optional cache initialization poisons the class, fatal errors are swallowed, fallback reads stop running, or lifecycle methods dereference a missing manager.
  • Rollback or disable path: Revert the maintainer follow-up commit to restore the original static implementation.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: OpenAI Codex assisted with the maintainer follow-up, regression tests, and verification; the resulting diff was reviewed locally.

@HandSonic
HandSonic force-pushed the fix/1724-cache-manage-throwable branch from 307a5d2 to 73f0beb Compare July 21, 2026 08:14
@openai0229

openai0229 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

The fuzzyDelete() initialization guard looks correct. However, please narrow the catch (Throwable) clause. Catching every Throwable also swallows errors such as OutOfMemoryError, StackOverflowError, and ThreadDeath, after which the application should not continue as if only the optional cache failed. If the goal is to handle NoClassDefFoundError, use a targeted clause such as catch (Exception | LinkageError e) or catch (Exception | NoClassDefFoundError e). The original StateTransitionException shown in the referenced historical issue is already covered by Exception in the current code, so a current-version reproduction or focused test for the intended Error path would also help validate this change.

@HandSonic
fix(cache): catch Throwable in CacheManage static init to prevent cla…
c3bdc93 
…ss poisoning (OtterMind#1724)

The static initializer only caught Exception, missing Errors like
NoClassDefFoundError. Once the init failed with an Error, the class
was permanently poisoned and cache was silently disabled.

Also add init guard to fuzzyDelete() to prevent NPE when cache init
failed and fuzzyDelete is called externally.
@HandSonic
HandSonic force-pushed the fix/1724-cache-manage-throwable branch from 73f0beb to c3bdc93 Compare July 21, 2026 12:24
@HandSonic

HandSonic commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Fixed. Changed to , and added an guard at the beginning of to prevent accessing when cache is not initialized.

@HandSonic

HandSonic commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Fixed. Changed catch (Throwable e) to catch (Exception | LinkageError e), and added an if (!init) return; guard at the beginning of fuzzyDelete() to prevent accessing cacheManager when cache is not initialized.

openai0229
openai0229 requested changes Jul 24, 2026
View reviewed changes

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The narrowed Exception | LinkageError boundary is a better direction than catching all Throwable, but this still needs a focused regression test that forces cache initialization to fail and verifies the class remains usable in the intended degraded mode.

Please also coordinate with #1925: both PRs add the same fuzzyDelete initialization guard, so the duplicate change should live in only one focused PR. After resolving that overlap, update the branch with main and rerun the required checks.

@openai0229 openai0229 mentioned this pull request Jul 24, 2026
Merged
openai0229
openai0229 requested changes Jul 24, 2026
View reviewed changes

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewing the current head: this should not be merged without a current reproduction and focused test.

The failure reported in historical issue #1724 was an Ehcache StateTransitionException, which is already covered by the catch (Exception e) present on current main. This PR additionally catches every LinkageError and adds a degraded-mode guard, but it does not demonstrate a current 5.3.x failure that requires that broader boundary or test initialization failure behavior.

Issue #1724 has been closed as outdated, the branch is 54 commits behind main, and the fuzzyDelete guard overlaps #1925. I recommend closing this PR. A new change should start from a current-version reproduction, define which initialization failures are safe to degrade, and include a regression test.

@openai0229 openai0229 changed the title fix(cache): catch Throwable in CacheManage static init (#1724) fix(cache): degrade safely when optional cache initialization fails Jul 24, 2026
openai0229
openai0229 approved these changes Jul 24, 2026
View reviewed changes

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer follow-up completed. Optional cache initialization now has an explicit unavailable state, catches only Exception and LinkageError, preserves fatal Error propagation, and the fallback/lifecycle regression tests pass locally.

@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 24, 2026
@openai0229
openai0229 merged commit 9d71d83 into OtterMind:main Jul 24, 2026
16 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@openai0229 openai0229 openai0229 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Chat2DB Community
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

refactor(cache): make optional cache initialization degradation testable

2 participants

@HandSonic @openai0229